Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: store confound timeseries data #1166

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

psadil
Copy link
Contributor

@psadil psadil commented Dec 5, 2023

Closes #992

The timeseries data are stored in a tsv file matching [input_bids]_timeseries.tsv, with an accompanying json sidecar [input_bids]_timeseries.json

Columns contain 6 motion parameters, DVARS, AFNI’s outlier ratio, and AFNI’s quality index

(Sorry that this took so long to push through!)

@psadil
Copy link
Contributor Author

psadil commented Dec 5, 2023

I see that flake8 tests are failing due to more than just the changes in this pull (and that I've contributed to the mess). Should I aim to get the flake8 tests to pass?

Also, not sure what to make of the failing T1w test. The values are close. Has something recently been modified that would change numerical precision?

@effigies
Copy link
Member

effigies commented Dec 5, 2023

I would rebase on the latest master, which is linting clean. Then you should only have issues in your code to address.

@psadil psadil changed the title store confound timeseries data ENH: store confound timeseries data Dec 5, 2023
mriqc/interfaces/functional.py Outdated Show resolved Hide resolved
mriqc/interfaces/functional.py Outdated Show resolved Hide resolved
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
psadil and others added 2 commits December 5, 2023 14:41
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@psadil
Copy link
Contributor Author

psadil commented Dec 5, 2023

Thanks for the speedy review!

I don't think that the ci/circleci T1w failure is related to this pull request, but let me know if I should look into it as a part of this.

@effigies
Copy link
Member

effigies commented Dec 5, 2023

No, that's unrelated.

@effigies effigies merged commit 41fdc70 into nipreps:master Dec 6, 2023
13 of 14 checks passed
@psadil psadil deleted the enh/store-qc-timeseries branch December 6, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not discard confound timeseries and preserve all data?
2 participants